Skip to content

Conversation

@Caio-Nogueira
Copy link
Contributor

Python Workflows call into the workflows wrapped binding via FFI. Since the binding is a Js proxy, method signatures from binding object are not automatically converted into snake case (as opposed to other JS methods that get converted by Pyodide).

This PR fixes this problem by adding new methods with snake cased signatures.

The alternative to this would be converting methods in runtime, via the Python entrypoint helper. However, that's not really feasible because we don't have a way to distinguish the workflow wrapper from another userland object.

@Caio-Nogueira Caio-Nogueira requested a review from a team as a code owner October 23, 2025 15:48
@codspeed-hq
Copy link

codspeed-hq bot commented Oct 23, 2025

CodSpeed Performance Report

Merging #5398 will not alter performance

Comparing caio/python-snake-case-signatures (1b5be7f) with main (4c1c293)

Summary

✅ 33 untouched
⏩ 9 skipped1

Footnotes

  1. 9 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

Copy link
Contributor

@dom96 dom96 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, should this not only be declared for Python Workers? Might make sense to disallow the camelCase there too after a compat flag.

@Caio-Nogueira Caio-Nogueira force-pushed the caio/python-snake-case-signatures branch from be89f99 to 5ef66bb Compare October 24, 2025 11:02
@Caio-Nogueira Caio-Nogueira requested review from a team as code owners October 24, 2025 11:02
Copy link
Contributor

@LuisDuarte1 LuisDuarte1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM from the Workflows team

Copy link
Contributor

@dom96 dom96 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. But need @hoodmane to take a look as he has some concerns around how changes to the SDK will affect our snapshotting.

@Caio-Nogueira Caio-Nogueira force-pushed the caio/python-snake-case-signatures branch from 5ef66bb to 1b5be7f Compare October 27, 2025 11:16
@Caio-Nogueira Caio-Nogueira merged commit 5402d95 into main Oct 28, 2025
32 of 33 checks passed
@Caio-Nogueira Caio-Nogueira deleted the caio/python-snake-case-signatures branch October 28, 2025 20:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants